Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cli: add petstore lint test to fix spectral ESM bundling error #8055

Draft
wants to merge 8 commits into
base: develop
Choose a base branch
from
Draft

cli: add petstore lint test to fix spectral ESM bundling error #8055

wants to merge 8 commits into from

Conversation

jackkav
Copy link
Contributor

@jackkav jackkav commented Oct 9, 2024 •
edited
Loading

motivation: reproduce and fix bundling error when linting with custom spec

error we want to fix

 FATAL  require() of ES Module /snapshot/insomnia/node_modules/jsep/dist/cjs/jsep.cjs.js from /snapshot/insomnia/node_modules/simple-eval/dist/index.js not supported.            
jsep.cjs.js is treated as an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which declares all .js files in that package scope as ES modules.
Instead either rename jsep.cjs.js to end in .cjs, change the requiring code to use dynamic import() which is available in all CommonJS modules, or change "type": "module" to "type": "commonjs" in /snapshot/insomnia/node_modules/jsep/package.json to treat all .js files as CommonJS (using .mjs for all ES modules instead).

contributing factors

  • vercel/pkg
  • spectral has partial esm deps
  • esbuild
    one of these three is excluding required spectral sub-dependancies

working theory

when linting with a custom .spectral.yaml spectral will read this file and create a "virtual spectral.js" which pulls in dependencies from the working directory. We would like these deps to be bundled with inso but the bundleAndLoadRuleset logic is expecting these things to be available in the working directory.

try ideas

  • bump spectral ⛔
  • bump pkg ⛔
  • assert the required modules are included in esbuild out ⛔
  • use a esbuild plugin to modify somehow ⛔
  • pin jsep to 1.1.2 ⛔
  • esbuild flag?
  • run the step outside the repo
  • run npm i jsep next to the spec
  • delete lint spec (in theory does deck have spec linting?)
  • load the custom spec ourselves(remove bundleAndLoadRuleset)
  • fork spectral-core replacing simple-eval with jsep function / simpler-eval
  • use a tsconfig path alias, shrug
  • pre bundle a dist of spectral
  • use typescript to build instead of esbuild

notes

  • package will fail if jsep is missing from the index.js
  • modifications to jsep path in index.js do not appear to be reflected in the final binary
  • when running packaging against only the index.js it will search for external node_modules which could explain above
  • or possibly the generated spectral.js in the folder where our api spec is ignoring the build contents

summary: pkg is reading spectral deps from node_modules rather than the index.js

warning from vercel/pkg related to it pulling in react files which are unused

> Warning Cannot resolve 'moduleName'
  /Users/[email protected]/git/insomnia/packages/insomnia-inso/dist/index.js
  Dynamic require may fail at run time, because the requested file
  is unknown at compilation time and not included into executable.
  Use a string literal as an argument for 'require', or leave it
  as is and specify the resolved file name in 'scripts' option.
> Warning Cannot resolve 'filePath'
  /Users/[email protected]/git/insomnia/packages/insomnia-inso/dist/index.js
  Dynamic require may fail at run time, because the requested file
  is unknown at compilation time and not included into executable.
  Use a string literal as an argument for 'require', or leave it
  as is and specify the resolved file name in 'scripts' option.
> Warning Failed to make bytecode node20-arm64 for file /snapshot/insomnia/node_modules/@isaacs/cliui/node_modules/string-width/index.js
> Warning Failed to make bytecode node20-arm64 for file /snapshot/insomnia/node_modules/@isaacs/cliui/node_modules/strip-ansi/index.js
> Warning Failed to make bytecode node20-arm64 for file /snapshot/insomnia/node_modules/@isaacs/cliui/node_modules/wrap-ansi/index.js
> Warning Failed to make bytecode node20-arm64 for file /snapshot/insomnia/node_modules/@isaacs/cliui/node_modules/ansi-regex/index.js
> Warning Failed to make bytecode node20-arm64 for file /snapshot/insomnia/node_modules/@isaacs/cliui/node_modules/ansi-styles/index.js

closes INS-4120

@jackkav jackkav changed the title cli: add petstore lint test cli: add petstore lint test to fix spectral ESM bundling error Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@jackkav